-
Notifications
You must be signed in to change notification settings - Fork 90
Refactor the natural numbers and integers up to present code standards #1568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll admit it's not obvious to me that these files on the standard finite types belong in elementary-number-theory and not univalent-combinatorics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to me either
Also, I broke several files down into more separate sections. |
src/elementary-number-theory/exponentiation-natural-numbers.lagda.md
Outdated
Show resolved
Hide resolved
I'm a bit worried that these changes will make it considerably harder to get #1211 merged down the road. But since that PR has been stale for a while and there are likely other merged changes that already make this difficult I'm not sure it should be a blocker. I would be incredibly grateful if someone were to comb through that PR and try and get the more finished parts of it merged though, such as the well-ordering principle of the natural numbers and divisibility among many others. |
I'd like to get some feedback from another maintainer before this PR is potentially merged. Preferrably @EgbertRijke, but otherwise @VojtechStep, since I expect the former will not respond. From what I've seen so far, this PR mostly abstracts definitions and fixes a few names. Are these changes necessary for your continued process @lowasser, or could we potentially icebox this PR until progress has been made to merge the other one? Otherwise, should we consider #1211 discontinued and not take it into consideration, @VojtechStep? Keep in mind I have not checked exactly how much overlap there are between these two PRs. |
I don't think #1211 is becoming mergeable any time soon (it never typechecked, it's been stale for 8 months, and in any case a +17k-3k PR would be very hard to review in one go), and I don't think we should freeze |
These are not necessary changes, just things that have irritated me. |
Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
src/elementary-number-theory/poset-of-natural-numbers-ordered-by-divisibility.lagda.md
Outdated
Show resolved
Hide resolved
src/elementary-number-theory/relatively-prime-natural-numbers.lagda.md
Outdated
Show resolved
Hide resolved
src/elementary-number-theory/relatively-prime-integers.lagda.md
Outdated
Show resolved
Hide resolved
src/elementary-number-theory/divisibility-natural-numbers.lagda.md
Outdated
Show resolved
Hide resolved
I guess I'm just struggling to accept that #1211 is again not going to get merged, and this PR puts a final nail in its coffin. I'll okay these changes for merging. |
Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
It's worth emphasizing that the software practice I've been trained in, at least, emphasizes smaller PRs that try to do one thing. Not that this PR is a great example of that, but several separate changes, even ones with interdependencies, are easier to merge, less painful if they have to be dropped for whatever reason, faster to understand and review and make progress on... I can see if I can try to rescue bits of it, though. |
Maybe I should've done the natural numbers and the integers separately? If so, I'll split it...
Changes include:
is-equiv-is-invertible